Skip to content

ln: add WASI support via symlink_path#11713

Open
DePasqualeOrg wants to merge 6 commits intouutils:mainfrom
DePasqualeOrg:wasi-ln-symlink
Open

ln: add WASI support via symlink_path#11713
DePasqualeOrg wants to merge 6 commits intouutils:mainfrom
DePasqualeOrg:wasi-ln-symlink

Conversation

@DePasqualeOrg
Copy link
Copy Markdown
Contributor

This PR replaces the error-returning WASI stub with a real symlink implementation using std::os::wasi::fs::symlink_path.

Changes

  • Import std::os::wasi::fs::symlink_path as symlink alongside the existing unix and windows symlink imports
  • Remove the WASI fn symlink stub that returned io::ErrorKind::Unsupported
  • Add #![cfg_attr(target_os = "wasi", feature(wasi_ext))] for access to std::os::wasi::fs

Limitations

Relative symlink targets work. Absolute symlink targets (e.g., ln -s /tmp/foo link) are rejected by WASI runtimes due to capability-based sandboxing — this is a platform constraint, not a code limitation.

Build requirements

Rust nightly (for std::os::wasi::fs, gated with feature(wasi_ext)).

Testing

Verified working on Wasmer and Wasmtime with relative symlink targets.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/pr/bounded-memory is now being skipped but was previously passing.
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented Apr 8, 2026

error[E0554]: `#![feature]` may not be used on the stable release channel
 --> src/uu/ln/src/ln.rs:6:33
  |
6 | #![cfg_attr(target_os = "wasi", feature(wasi_ext))]
  |                                 ^^^^^^^^^^^^^^^^^

@DePasqualeOrg
Copy link
Copy Markdown
Contributor Author

I added a new commit to address the CI failures.

The WASI CI uses stable Rust, but std::os::wasi::fs::symlink_path requires the unstable feature(wasi_ext) gate. This commit replaces it with libc::symlink (available via wasi-libc), removes the feature(wasi_ext) gate, and adds libc as a dependency. Same functionality, compiles on stable.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/rm/isatty (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/pr/bounded-memory is now being skipped but was previously passing.
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!

pub fn symlink<P1: AsRef<Path>, P2: AsRef<Path>>(src: P1, dst: P2) -> io::Result<()> {
use std::os::wasi::ffi::OsStrExt;

let src_c = CString::new(src.as_ref().as_os_str().as_bytes())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use libc or rustix instead?
i really would like to avoid CString

@sylvestre
Copy link
Copy Markdown
Contributor

could you please add a test?
(or enable one for wasi)
thanks

@sylvestre
Copy link
Copy Markdown
Contributor

still need a test :)

@DePasqualeOrg
Copy link
Copy Markdown
Contributor Author

It's now using rustix instead.

It looks like the existing integration tests cover symlink creation, but CI has skip-tests: true for wasm32-wasip1, since there's no WASI runtime.

@sylvestre
Copy link
Copy Markdown
Contributor

@oech3

This comment was marked as off-topic.

@sylvestre
Copy link
Copy Markdown
Contributor

@oech3 please don't use this PR for this discussion

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 8, 2026

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/expand/bounded-memory is now passing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!

@DePasqualeOrg
Copy link
Copy Markdown
Contributor Author

I have enabled integration tests for WASI in #11717. I suggest that that PR be reviewed first, and after it's merged I can enable symlink tests in this PR.

On wasm32-wasip1, std::os::unix::fs::symlink is not available, but
WASI preview 1 provides path_symlink which Rust exposes as
std::os::wasi::fs::symlink_path. Import it under the symlink alias
so the existing call site works without changes.

Follows the same pattern as cp.rs for enabling wasi_ext.
Avoids Path::exists() returning false for a live symlink whose target
stat fails (observed under wasmtime), and also keeps a dangling .~N~
symlink from being overwritten by a --backup=numbered rename.
- test_symlink_to_dir_2args uses an absolute host tmpdir path that
  isn't visible inside the WASI sandbox.
- test_ln_non_utf8_paths requires non-UTF-8 filenames, which WASI
  forbids.
- test_relative_src_already_symlink hits a read_link-on-absolute-path
  failure specific to wasmtime launched through cargo test's spawn.
Adds test_ln:: to the wasmtime integration test list and drops ln from
the pending-tools TODO. Documents the read_link-on-absolute-paths gap
in wasi-test-gaps.md.
@DePasqualeOrg
Copy link
Copy Markdown
Contributor Author

Now that #11717 is merged, I've added ln to the list of tools covered by the WASI integration tests. Most of the test_ln cases pass as is; three of them are skipped via #[cfg_attr(wasi_runner, ignore)]:

  • test_symlink_to_dir_2args: passes an absolute host tmpdir path, which the sandbox doesn't map.
  • test_ln_non_utf8_paths: WASI requires UTF-8 argv/filenames.
  • test_relative_src_already_symlink: fs::read_link on an absolute path (/file2) returns EPERM under wasmtime, but only when the binary is launched through std::process::Command from the cargo-test harness. Direct wasmtime invocation and a minimal external spawner both work. fs::symlink_metadata on the same path always works. This reproduces on macOS and Linux. A likely fix is to keep uucore::fs::canonicalize in relative-path form instead of joining onto the absolute current_dir.

One previously failing test (test_symlink_existing_backup) is fixed rather than skipped: uucore::backup_control now uses fs::symlink_metadata(...).is_ok() instead of Path::exists() to probe for .~N~ backups. exists() follows symlinks, so it was returning false for a live .~1~ symlink whose target stat fails under wasmtime, causing ln's --backup=nil path to fall through to the simple-backup form instead of bumping to .~2~. Using symlink_metadata also correctly treats dangling .~N~ symlinks as existing, which matters under --backup=numbered where the old check would otherwise overwrite the dangling slot. (For reference, GNU's backupfile.c avoids the question entirely by scanning the directory with readdir rather than stat-ing each candidate.)

Testing

Ran on macOS locally with RUSTFLAGS=--cfg wasi_runner: 47 passed, 2 ignored (the third skipped test is Linux-only and doesn't compile on macOS). Also verified on Ubuntu 24.04 via Docker by running a test binary compiled without the cfg, so the annotations weren't applied; every test_ln case passed except the three we're skipping, confirming those are the only incompatibilities.

@github-actions
Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tty/tty-eof (fails in this run but passes in the 'main' branch)
Congrats! The gnu test tests/dd/no-allocate is now passing!
Congrats! The gnu test tests/printf/printf-surprise is now passing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants